Skip to content

Superseded by #3 - #2

Closed
zacala1 wants to merge 8 commits into
mainfrom
claude/festive-shannon-gqed26
Closed

zacala1 wants to merge 8 commits into
mainfrom
claude/festive-shannon-gqed26

Conversation

@zacala1

@zacala1 zacala1 commented Oct 3, 2026 •

Copy link
Copy Markdown
Owner

Superseded by #3: same changes, moved to the branch fix/lock-safety-and-crash-recovery with commits authored under the repository owner's identity. Closing this one.

claude added 2 commits October 3, 2026 17:57
… support

Code review findings, fixed:

* Linux: a live lock owner was reported as an orphan, so any waiter stole
  its write lock. Process.StartTime is derived per process from a wall-clock
  boot-time snapshot, so the value an owner records and the value another
  process computes for it differ by milliseconds and never compare equal.
  Record and compare /proc/<pid>/stat starttime (kernel ticks) instead.
  Values written by 3.0.0 (negative DateTime binaries) are treated as
  unknown and fall back to the PID-only check.

* Orphan recovery only probed the owner at the start of a wait and at 75% of
  the timeout, so a wait with Timeout.InfiniteTimeSpan never recovered from
  an owner that died later. Probe every 250 ms instead.

* Dispose() unmapped the header while another thread was still spinning in
  TryAcquireWriteLock/TryAcquireReadLock, which is a process-fatal
  AccessViolationException. Lock waiters now register with Dispose(), leave
  with ObjectDisposedException, and Dispose() waits for them before unmapping.

* ReleaseWriteLock silently ignored a release from a non-owner thread, so
  `await` inside a lock scope left the lock held forever with no diagnostic.
  It now throws SynchronizationLockException, as do the StructuredMemory
  WriteLock/ReadLock guards when disposed on another thread (before touching
  any state, so the owning thread can still release correctly).

* TypeLayoutFingerprint used Marshal.SizeOf/OffsetOf, which reject generic
  types, so ValueTuple/KeyValuePair<,> failed in every typed container.
  Fall back to a managed-layout descriptor for those types. Fingerprints of
  types that already worked are byte-for-byte unchanged (pinned by a test),
  so 3.0.0 and newer processes can still share regions.

Tests: cross-process regression tests (live owner, infinite and finite wait
recovery) with a new hold_write_lock worker role, Dispose-with-waiter,
guard-on-another-thread, generic struct and pinned fingerprint tests. Two
tests that asserted the old "silently ignored" behaviour now expect the
exception.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
* OrphanLockTimeout now defaults to TimeSpan.Zero. The previous default of
  30 s let a waiter take the write lock from a healthy process that held it
  longer than that (a long transaction, a paused debugger), breaking mutual
  exclusion. Recovery of a lock whose owner process has exited is unaffected
  and stays on by default. DefaultOrphanLockTimeout keeps its value and is
  documented as the suggested value when opting in.

* Read locks are not attributed to an owner, so a process that dies holding
  one leaves the shared reader count above zero forever; on Linux the
  /dev/shm file even outlives every user. This cannot be detected
  automatically, so give operators explicit tools:
  - MemoryRegion.ForceResetLocks() / StructuredMemory<T>.ForceResetLocks()
    clear the write lock and the reader count.
  - LockOwnerInfo.ReaderCount exposes the count for diagnosis.
  - MemoryRegion.Remove(name, options) deletes the backing storage (Linux
    /dev/shm file or an explicit FilePath) so a region left unusable by a
    crashed creator, or one with an unwanted capacity or element type, can be
    recreated. The initialization timeout message now points to it.

README documents both the new default and the recovery procedure.

Tests: cross-process dead-reader scenario (new hold_read_lock worker role),
ForceResetLocks for MemoryRegion and StructuredMemory, Remove for Linux
regions, file-backed regions and a region stuck in the initializing state.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
@zacala1 zacala1 changed the title Fix Linux lock-owner detection, lock-wait robustness, and generic struct support Fix Linux lock-owner detection and lock-wait robustness; make lock takeover opt-in; add crash-recovery tools Oct 3, 2026
Disposing a region while another thread is inside Read/Write or a typed
queue unmaps memory that thread is still using, which terminates the
process with an AccessViolationException. Measured with three threads busy
polling a ConcurrentQueue on a 4-core machine, Dispose killed the process
in 12 of 12 runs of 400 races each.

Tracking in-flight calls would close the race but is not affordable on the
lock-free paths: two interlocked operations per call made a single-thread
queue round trip about 9x slower (3.3 ns -> 31 ns), two threads exchanging
items about 17x slower (about 70 M -> 4 M msgs/s) and MemoryRegion.Read
about 2x slower. Striping the counter per CPU did not help.

Instead, MemoryRegion.Dispose now keeps the mapping for
MemoryRegion.DisposeGracePeriod (default 10 ms, process-wide, Zero
disables) after the instance is marked disposed. New calls already fail
with ObjectDisposedException, so the pause lets calls that were past their
check finish. Measured: 12 of 12 runs of 400 races survive with the
default, and 8 oversubscribed pollers on 4 cores still failed in 1 of 8
runs (0 of 8 at 25 ms). It is a mitigation, not a guarantee, and says so
in the XML docs and the README: the only complete protection is to stop
and join every thread that uses an instance before disposing it.

Tests: the grace period setting, that Dispose honours it, and a busy-poll
Dispose race that kills the process without the grace period.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
@zacala1 zacala1 changed the title Fix Linux lock-owner detection and lock-wait robustness; make lock takeover opt-in; add crash-recovery tools Fix Linux lock-owner detection and lock-wait robustness; opt-in lock takeover; crash-recovery tools; narrow Dispose races Oct 4, 2026
claude added 5 commits October 4, 2026 04:49
* SharedArray did not dispose its region when the header check failed
  (another element type or length), so the mapping and, on Linux, its file
  descriptor stayed open until the finalizer ran. The other containers
  already clean up; do the same here.

* SharedArray and StructuredMemory expose no statistics, yet their region
  updated its read/write counters with interlocked operations on every
  access. Turn the counters off for these internal regions: no observable
  change, SharedArray<long> get 24 -> 18 ns and set 15.6 -> 5.9 ns.

* Every automatically locked StructuredMemory call (values wider than
  8 bytes, strings, blobs, arrays) allocated 104 bytes: a new delegate for
  the lock guard (method group conversion) plus a Stopwatch instance inside
  the lock wait. Cache the delegates per instance and time the lock waits
  with Stopwatch timestamps. Measured 104 -> 0 bytes per call and
  Write<Guid> 311 -> 190 ns.

Tests: failed open leaves no extra file descriptor (Linux), and automatically
locked access allocates nothing per call.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
ConcurrentMessageQueue, SingleProducerByteStream, SharedArray and
StructuredMemory had finalizers that only set a flag or disposed a
MemoryHandle, which wraps an unmanaged pointer and owns nothing. They made
every instance finalizable, which costs an extra GC promotion, without
freeing anything: when Dispose is never called, the MemoryRegion's own
finalizer already unmaps the memory.

Also drop GC.SuppressFinalize from SingleProducerQueue and ConcurrentQueue,
which never had a finalizer.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
SingleProducerQueue, ConcurrentQueue and ConcurrentMessageQueue each carried
a private bit-twiddling copy of RoundUpToPowerOf2. Use BitOperations through
one internal helper instead. Behaviour is unchanged except that an
out-of-range capacity now reports the parameter name "capacity" and the
offending value instead of "value", and the capacity-mismatch check computes
the rounded value once.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
Both types wrote their header fields and then the magic with two plain
region writes, and read the whole header in one copy while polling for the
magic. MemoryRegion.Write/Read do no fencing, and a single copy gives no
ordering between the magic and the bytes after it, so on a weakly ordered
CPU (ARM) an opener could see the magic before the fields and fail with a
spurious "different format" error. The queues already publish their magic
last with a release write.

Insert a full fence before the magic is written, and on the reader poll the
4-byte magic alone, then fence and read the fields. This is a no-op on x86,
and the ordering cannot be exercised on the x86 machine used here; the
existing open/create tests cover the unchanged behaviour.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
* MPMC_BurstTraffic_ShouldHandleSpikes: the consumer gave up after 10,000
  empty polls, which can be used up in microseconds when it starts before
  the producer. The producer then spun forever on a full queue and the test
  hung (about 45% of isolated runs). The consumer now waits for the full
  burst, and both sides stop with an error after 30 s instead of hanging.

* MPMC_HighContention_ShouldMaintainIntegrity: same give-up heuristic, plus
  thread-pool starvation. Four spinning producers on Task.Run occupy every
  pool worker of a small machine, the consumers cannot start for seconds,
  and the producers give up on the full queue ("stuck at message 256",
  which is the slot count). It failed 25 of 25 isolated runs in a fresh
  process and only passed inside the full suite because earlier tests had
  already grown the pool. Producers and consumers now run on dedicated
  threads (TaskCreationOptions.LongRunning) and consumers wait for the full
  count with a deadline.

Both pass 15 of 15 isolated runs afterwards. No library change.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01JHFt4htvPpb8R3ZeepEVF8
@zacala1 zacala1 changed the title Fix Linux lock-owner detection and lock-wait robustness; opt-in lock takeover; crash-recovery tools; narrow Dispose races Superseded by #3 Oct 5, 2026
@zacala1 zacala1 closed this Oct 5, 2026
@zacala1
zacala1 deleted the claude/festive-shannon-gqed26 branch October 7, 2026 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants